From ef28cd2d97a624aff5320c6ea71f6ca2b06c606c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 6 Aug 2014 17:50:55 -0700 Subject: [PATCH] Robustly run binaries and tests after compilation These were previously just run by executing the raw binary, but this didn't ensure that any of the necessary paths were in the dylib search path for the host platform. This commit enhances the return type of `compile_targets` to have information about the result of compilation, along with the ability to spawn correctly configured processes. This primarily fixes `cargo test` and `cargo run` whenever dynamic dependencies are involved, but it also fixes the two commands whenever there's a native dynamic dependency involved as well. --- src/cargo/ops/cargo_compile.rs | 2 +- src/cargo/ops/cargo_run.rs | 4 +- src/cargo/ops/cargo_rustc/compilation.rs | 60 ++++++++++++++++++++++++ src/cargo/ops/cargo_rustc/context.rs | 7 ++- src/cargo/ops/cargo_rustc/fingerprint.rs | 30 +++++++++--- src/cargo/ops/cargo_rustc/mod.rs | 46 +++--------------- src/cargo/ops/cargo_test.rs | 33 +++++-------- src/cargo/ops/mod.rs | 2 +- tests/test_cargo_run.rs | 31 ++++++++++++ tests/test_cargo_test.rs | 44 ++++++++++++++++- tests/test_cargo_version.rs | 3 +- 11 files changed, 189 insertions(+), 73 deletions(-) create mode 100644 src/cargo/ops/cargo_rustc/compilation.rs diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 5bb076a35..921bf5841 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -46,7 +46,7 @@ pub struct CompileOptions<'a> { pub fn compile(manifest_path: &Path, options: &mut CompileOptions) - -> CargoResult>> { + -> CargoResult { let CompileOptions { update, env, ref mut shell, jobs, target } = *options; let target = target.map(|s| s.to_string()); diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index 91879706a..67a1f7c2f 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -16,13 +16,13 @@ pub fn run(manifest_path: &Path, try!(src.update()); let root = try!(src.get_root_package()); - try!(ops::compile(manifest_path, options)); + let compile = try!(ops::compile(manifest_path, options)); let exe = manifest_path.dir_path().join("target").join(root.get_name()); let exe = match exe.path_relative_from(&os::getcwd()) { Some(path) => path, None => exe, }; - let process = process(exe).args(args); + let process = compile.process(exe).args(args); try!(options.shell.status("Running", process.to_string())); Ok(process.exec().err()) diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs new file mode 100644 index 000000000..4af3050cb --- /dev/null +++ b/src/cargo/ops/cargo_rustc/compilation.rs @@ -0,0 +1,60 @@ +use std::collections::HashMap; +use std::dynamic_lib::DynamicLibrary; +use std::os; + +use core::PackageId; +use util; + +/// A structure returning the result of a compilation. +pub struct Compilation { + /// All libraries which were built for a package. + /// + /// This is currently used for passing --extern flags to rustdoc tests later + /// on. + pub libraries: HashMap>, + + /// An array of all tests created during this compilation. + pub tests: Vec, + + /// An array of all binaries created. + pub binaries: Vec, + + /// All directires for the output of native build commands. + /// + /// This is currently used to drive some entries which are added to the + /// LD_LIBRARY_PATH as appropriate. + pub native_dirs: HashMap, + + /// Root output directory (for the local package's artifacts) + pub root_output: Path, + + /// Output directory for rust dependencies + pub deps_output: Path, +} + +impl Compilation { + pub fn new() -> Compilation { + Compilation { + libraries: HashMap::new(), + native_dirs: HashMap::new(), + root_output: Path::new("/"), + deps_output: Path::new("/"), + tests: Vec::new(), + binaries: Vec::new(), + } + } + + /// Prepares a new process with an appropriate environment to run against + /// the artifacts produced by the build process. + pub fn process(&self, cmd: T) -> util::ProcessBuilder { + let mut search_path = DynamicLibrary::search_path(); + for dir in self.native_dirs.values() { + search_path.push(dir.clone()); + } + search_path.push(self.root_output.clone()); + search_path.push(self.deps_output.clone()); + let search_path = os::join_paths(search_path.as_slice()).unwrap(); + util::process(cmd).env(DynamicLibrary::envvar(), + Some(search_path.as_slice())) + } +} diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 95e507075..56d969fd2 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -5,7 +5,7 @@ use core::{SourceMap, Package, PackageId, PackageSet, Resolve, Target}; use util; use util::{CargoResult, ChainError, internal, Config, profile}; -use super::{Kind, KindPlugin, KindTarget}; +use super::{Kind, KindPlugin, KindTarget, Compilation}; use super::layout::{Layout, LayoutProxy}; #[deriving(Show)] @@ -21,6 +21,7 @@ pub struct Context<'a, 'b> { pub config: &'b mut Config<'b>, pub resolve: &'a Resolve, pub sources: &'a SourceMap, + pub compilation: Compilation, env: &'a str, host: Layout, @@ -59,6 +60,7 @@ impl<'a, 'b> Context<'a, 'b> { target_exe: target_exe, host_dylib: host_dylib, requirements: HashMap::new(), + compilation: Compilation::new(), }) } @@ -122,6 +124,9 @@ impl<'a, 'b> Context<'a, 'b> { self.build_requirements(pkg, target, Target, &mut HashSet::new()); } + self.compilation.root_output = self.layout(KindTarget).proxy().dest().clone(); + self.compilation.deps_output = self.layout(KindTarget).proxy().deps().clone(); + Ok(()) } diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 2acf5c082..0626ec068 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -65,14 +65,29 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, }; let is_rustc_fresh = try!(is_fresh(&old_loc, rustc_fingerprint.as_slice())); - let layout = cx.layout(kind); + let (old_root, root) = { + let layout = cx.layout(kind); + (layout.old_root().clone(), layout.root().clone()) + }; let mut pairs = vec![(old_loc, new_loc.clone())]; if !target.get_profile().is_doc() { pairs.push((old_dep_info, new_dep_info)); - pairs.extend(cx.target_filenames(target).iter().map(|filename| { + + for filename in cx.target_filenames(target).iter() { let filename = filename.as_slice(); - ((layout.old_root().join(filename), layout.root().join(filename))) - })); + let dst = root.join(filename); + pairs.push((old_root.join(filename), root.join(filename))); + + if target.get_profile().is_test() { + cx.compilation.tests.push(dst.clone()); + } else if target.is_bin() { + cx.compilation.binaries.push(dst.clone()); + } else if target.is_lib() && target.get_profile().is_compile() { + let pkgid = pkg.get_package_id().clone(); + cx.compilation.libraries.find_or_insert(pkgid, Vec::new()) + .push(root.join(filename)); + } + } } Ok(prepare(is_rustc_fresh && are_files_fresh, new_loc, rustc_fingerprint, @@ -117,9 +132,12 @@ pub fn prepare_build_cmd(cx: &mut Context, pkg: &Package) let new_fingerprint = mk_fingerprint(cx, &new_fingerprint); let is_fresh = try!(is_fresh(&old_loc, new_fingerprint.as_slice())); - let layout = cx.layout(kind); let pairs = vec![(old_loc, new_loc.clone()), - (layout.old_native(pkg), layout.native(pkg))]; + (cx.layout(kind).old_native(pkg), + cx.layout(kind).native(pkg))]; + + let native_dir = cx.layout(kind).native(pkg); + cx.compilation.native_dirs.insert(pkg.get_package_id().clone(), native_dir); Ok(prepare(is_fresh, new_loc, new_fingerprint, pairs)) } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index eeba58a9d..bc1936177 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -1,4 +1,4 @@ -use std::collections::{HashSet, HashMap}; +use std::collections::HashSet; use std::dynamic_lib::DynamicLibrary; use std::io::{fs, UserRWX}; use std::os; @@ -14,7 +14,10 @@ use self::job_queue::{JobQueue, StageStart, StageCustomBuild, StageLibraries}; use self::job_queue::{StageBinaries, StageEnd}; use self::context::{Context, PlatformRequirement, Target, Plugin, PluginAndTarget}; +pub use self::compilation::Compilation; + mod context; +mod compilation; mod fingerprint; mod job; mod job_queue; @@ -47,9 +50,9 @@ pub fn compile_targets<'a>(env: &str, targets: &[&'a Target], pkg: &'a Package, deps: &PackageSet, resolve: &'a Resolve, sources: &'a SourceMap, config: &'a mut Config<'a>) - -> CargoResult>> { + -> CargoResult { if targets.is_empty() { - return Ok(HashMap::new()); + return Ok(Compilation::new()) } debug!("compile_targets; targets={}; pkg={}; deps={}", targets, pkg, deps); @@ -86,12 +89,10 @@ pub fn compile_targets<'a>(env: &str, targets: &[&'a Target], pkg: &'a Package, cx.primary(); try!(compile(targets, pkg, &mut cx, &mut queue)); - let ret = build_return_map(&cx, pkg, deps); - // Now that we've figured out everything that we're going to do, do it! try!(queue.execute(cx.config)); - Ok(ret) + Ok(cx.compilation) } fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, @@ -455,36 +456,3 @@ fn pre_version_component(v: &Version) -> Option { Some(ret) } - -fn build_return_map(cx: &Context, root: &Package, deps: &PackageSet) - -> HashMap> { - let mut ret = HashMap::new(); - match cx.resolve.deps(root.get_package_id()) { - Some(mut my_deps) => { - for dep in my_deps { - let pkg = deps.iter().find(|p| p.get_package_id() == dep).unwrap(); - ret.insert(dep.clone(), build_paths(cx, pkg, false)); - } - } - None => {} - } - ret.insert(root.get_package_id().clone(), build_paths(cx, root, true)); - return ret; - - fn build_paths(cx: &Context, pkg: &Package, root: bool) -> Vec { - pkg.get_targets().iter().filter(|target| { - target.get_profile().is_compile() && target.is_lib() - }).flat_map(|target| { - let kind = if target.get_profile().is_plugin() { - KindPlugin - } else { - KindTarget - }; - let layout = cx.layout(kind); - cx.target_filenames(target).move_iter().map(|filename| { - let root = if root {layout.root()} else {layout.deps()}; - root.join(filename) - }).collect::>().move_iter() - }).collect() - } -} diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index cc7d22fb6..1609e0f62 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -3,7 +3,7 @@ use std::os; use core::Source; use sources::PathSource; use ops; -use util::{process, CargoResult, ProcessError}; +use util::{CargoResult, ProcessError}; pub fn run_tests(manifest_path: &Path, options: &mut ops::CompileOptions, @@ -12,27 +12,17 @@ pub fn run_tests(manifest_path: &Path, try!(source.update()); let package = try!(source.get_root_package()); - let compiled_libs = try!(ops::compile(manifest_path, options)); - - let mut exes: Vec = package.get_targets().iter().filter_map(|target| { - if !target.get_profile().is_test() { return None } - let root = package.get_root().join("target"); - let root = match target.get_profile().get_dest() { - Some(dest) => root.join(dest), - None => root, - }; - Some(root.join(target.file_stem())) - }).collect(); - exes.sort(); + let mut compile = try!(ops::compile(manifest_path, options)); + compile.tests.sort(); let cwd = os::getcwd(); - for exe in exes.iter() { + for exe in compile.tests.iter() { let to_display = match exe.path_relative_from(&cwd) { Some(path) => path, None => exe.clone(), }; try!(options.shell.status("Running", to_display.display())); - match process(exe).args(args).exec() { + match compile.process(exe).args(args).exec() { Ok(()) => {} Err(e) => return Ok(Some(e)) } @@ -47,18 +37,19 @@ pub fn run_tests(manifest_path: &Path, for (lib, name) in libs { try!(options.shell.status("Doc-tests", name)); - let mut p = process("rustdoc").arg("--test").arg(lib) - .arg("--crate-name").arg(name) - .arg("-L").arg("target/test") - .arg("-L").arg("target/test/deps") - .cwd(package.get_root()); + let mut p = compile.process("rustdoc") + .arg("--test").arg(lib) + .arg("--crate-name").arg(name) + .arg("-L").arg("target/test") + .arg("-L").arg("target/test/deps") + .cwd(package.get_root()); // FIXME(rust-lang/rust#16272): this should just always be passed. if args.len() > 0 { p = p.arg("--test-args").arg(args.connect(" ")); } - for (pkg, libs) in compiled_libs.iter() { + for (pkg, libs) in compile.libraries.iter() { for lib in libs.iter() { let mut arg = pkg.get_name().as_bytes().to_vec(); arg.push(b'='); diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 975ebcf2e..db47aadaf 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -1,7 +1,7 @@ pub use self::cargo_clean::clean; pub use self::cargo_compile::{compile, CompileOptions}; pub use self::cargo_read_manifest::{read_manifest,read_package,read_packages}; -pub use self::cargo_rustc::compile_targets; +pub use self::cargo_rustc::{compile_targets, Compilation}; pub use self::cargo_run::run; pub use self::cargo_new::{new, NewOptions}; pub use self::cargo_doc::{doc, DocOptions}; diff --git a/tests/test_cargo_run.rs b/tests/test_cargo_run.rs index 70ccd7714..51c4b22d5 100644 --- a/tests/test_cargo_run.rs +++ b/tests/test_cargo_run.rs @@ -82,3 +82,34 @@ test!(no_main_file { .with_stderr("`src/main.rs` must be present for \ `cargo run`\n")); }) + +test!(run_dylib_dep { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + path = "bar" + "#) + .file("src/main.rs", r#" + extern crate bar; + fn main() { bar::bar(); } + "#) + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + + [[lib]] + name = "bar" + crate-type = ["dylib"] + "#) + .file("bar/src/lib.rs", "pub fn bar() {}"); + + assert_that(p.cargo_process("cargo-run").arg("hello").arg("world"), + execs().with_status(0)); +}) diff --git a/tests/test_cargo_test.rs b/tests/test_cargo_test.rs index bf1d1cd11..b14043162 100644 --- a/tests/test_cargo_test.rs +++ b/tests/test_cargo_test.rs @@ -605,15 +605,42 @@ test!(test_dylib { [[lib]] name = "foo" crate_type = ["dylib"] + + [dependencies.bar] + path = "bar" "#) .file("src/lib.rs", " + extern crate bar; + + pub fn bar() { bar::baz(); } + #[test] - fn foo() {} + fn foo() { bar(); } + ") + .file("tests/test.rs", r#" + extern crate foo; + + #[test] + fn foo() { foo::bar(); } + "#) + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + + [[lib]] + name = "bar" + crate_type = ["dylib"] + "#) + .file("bar/src/lib.rs", " + pub fn baz() {} "); assert_that(p.cargo_process("cargo-test"), execs().with_status(0) .with_stdout(format!("\ +{compiling} bar v0.0.1 ({dir}) {compiling} foo v0.0.1 ({dir}) {running} target[..]test[..]foo-[..] @@ -622,6 +649,13 @@ test foo ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured +{running} target[..]test[..]test-[..] + +running 1 test +test foo ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured + {doctest} foo running 0 tests @@ -635,6 +669,7 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured\n\n\ assert_that(p.process(cargo_dir().join("cargo-test")), execs().with_status(0) .with_stdout(format!("\ +{fresh} bar v0.0.1 ({dir}) {fresh} foo v0.0.1 ({dir}) {running} target[..]test[..]foo-[..] @@ -643,6 +678,13 @@ test foo ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured +{running} target[..]test[..]test-[..] + +running 1 test +test foo ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured + {doctest} foo running 0 tests diff --git a/tests/test_cargo_version.rs b/tests/test_cargo_version.rs index 9fba04afa..03a72b8b0 100644 --- a/tests/test_cargo_version.rs +++ b/tests/test_cargo_version.rs @@ -1,5 +1,6 @@ use support::{project, execs}; use hamcrest::assert_that; +use cargo; fn setup() {} @@ -8,5 +9,5 @@ test!(simple { assert_that(p.cargo_process("cargo-version"), execs().with_status(0).with_stdout(format!("{}\n", - env!("CFG_VERSION")).as_slice())); + cargo::version()).as_slice())); }) -- 2.30.2